Skip to content

Make EventBus subscription failures observable (#74)#82

Merged
Faerkeren merged 1 commit into
mainfrom
fix/74-observable-subscription-failures
May 27, 2026
Merged

Make EventBus subscription failures observable (#74)#82
Faerkeren merged 1 commit into
mainfrom
fix/74-observable-subscription-failures

Conversation

@Faerkeren

Copy link
Copy Markdown
Contributor

Fixes #74.

Issue validity

Valid. EventBus.subscribe() schedules _ensure_subscription() with asyncio.ensure_future() after start, and _ensure_subscription catches every exception silently. The existing test test_subscribe_failure_logged_not_propagated explicitly locked in that behaviour. A listener could appear registered at the public API level while the underlying WebSocket subscribe never succeeded, so events would be silently missed.

Fix

Make subscription success/failure part of the API contract without breaking the existing fire-and-forget call sites.

  • New async subscribe_async(event_type, handler) / async unsubscribe_async(event_type, handler) propagate transport errors. subscribe_async rolls back the newly registered handler if the first subscribe for an event type fails, so the caller's view never diverges from the transport state.
  • The fire-and-forget subscribe() path now stores the scheduled task; callers can:
    • pending_subscription(event_type) -> awaitable in-flight subscribe task, or None.
    • subscription_failure(event_type) -> last recorded transport exception, cleared on a successful retry.
  • start() preserves the historic 'never raise on subscribe failure' contract (a single flaky event type should not abort startup), but now records the failure for inspection.
  • A done callback on the scheduled task moves the outcome (success / failure / cancellation) into the failure registry and clears the pending slot.

Tests

The existing test test_subscribe_failure_logged_not_propagated is replaced with test_subscribe_failure_during_start_is_recorded, which asserts the new contract. New coverage:

  • test_subscribe_post_start_failure_is_observable
  • test_subscribe_post_start_recovers_on_retry
  • test_subscribe_async_propagates_failure_and_rolls_back
  • test_subscribe_async_before_start_is_batched
  • test_subscribe_async_additional_handler_skips_ws_call
  • test_unsubscribe_async_propagates_transport_errors
  • test_unsubscribe_async_unknown_handler_is_noop
  • test_unsubscribe_async_does_not_release_ws_when_handlers_remain
  • test_pending_subscription_none_when_idle
  • test_cancelled_subscription_task_clears_pending

Checks run

  • ruff check src tests -> clean
  • ruff format --check src tests -> clean
  • mypy src -> clean (strict)
  • pytest tests/ --cov=haclient --cov-fail-under=95 -> 304 passed, 97.15% coverage

Backward compatibility

subscribe() / unsubscribe() / start() signatures and lifecycle are unchanged. Existing callers see no behavioural difference unless they opt in to subscribe_async / subscription_failure / pending_subscription.

Add awaitable subscribe_async/unsubscribe_async APIs that propagate
WebSocket transport errors to callers, and track outcomes of the
fire-and-forget subscribe/unsubscribe paths so callers can inspect
them.

- subscribe_async raises on transport failure and rolls back the
  newly registered handler so the caller's view stays consistent
  with the transport state.
- unsubscribe_async propagates errors from WebSocketPort.unsubscribe.
- subscribe() still does not raise, but the scheduled subscribe task
  is recorded and exposed via pending_subscription(); its outcome is
  cached in subscription_failure().
- start() preserves the historic 'never raise on subscribe failure'
  contract, but now records the failure for inspection so a flaky
  event type can be detected without changing startup semantics.

The existing test that locked in silent post-start failure behaviour
is replaced with assertions on the new contract, and new tests cover
post-start failure, recovery, rollback, multi-handler unsubscribe,
and cancellation.
@Faerkeren Faerkeren merged commit dca90ef into main May 27, 2026
12 checks passed
@Faerkeren Faerkeren deleted the fix/74-observable-subscription-failures branch May 27, 2026 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make EventBus subscription failures observable

1 participant